Add shared validator access validation and force flag by RedaRahmani · Pull Request #211 · malbeclabs/doublezero-offchain

RedaRahmani · 2025-11-24T19:46:06Z

Summary of Changes

This PR implements the behavior described in #155 for the passport CLI:

Extracts the validator access validation (gossip + leader scheduler checks for
primary and backup validators) into a shared helper module:
crates/solana-cli/src/command/passport/access_validation.rs.
Refactors prepare-validator-access to use this shared helper, preserving the
existing output and --force semantics.
Extends request-validator-access to:
- print the same cluster / validator info as prepare-validator-access,
- run the same validation checks before PDA/signature/tx submission, and
- support a new -f/--force flag so operators can override validation while
  still seeing the errors.
Wires the new module into crates/solana-cli/src/command/passport/mod.rs.

With this change, request-validator-access now mirrors the validation behavior
of prepare-validator-access:

Without --force: validation errors stop execution before the transaction is sent.
With --force: validation errors are printed as warnings and the transaction
is still submitted.

Testing Verification

I ran the CLI crate tests locally:

cargo test -p doublezero-solana-cli

RedaRahmani · 2025-11-24T19:50:53Z

Hi Team 👋

I’ve opened this PR to add the shared validator access validation logic and the -f/--force flag to request-validator-access, reusing the existing checks from prepare-validator-access.

When you have some time, I’d really appreciate a review and any feedback.
Happy to adjust anything in the implementation or tests if you prefer a different approach.

Thanks!

karl-dz

Thank you for your contribution. I left a few comments

RedaRahmani · 2025-11-24T21:13:21Z

Thanks a lot for the detailed review, @karl-dz !

I’ve updated the PR to:

move request_access back to an instance method on RequestValidatorAccessCommand,
hide the --force flag via #[arg(hide = true)],
inline the validation error printing into should_continue_after_validation,
and remove the duplicate tests from request_access.rs (tests live in access_validation.rs now).
If there’s anything else you’d like me to adjust or clean up, I’m happy to follow up.

karl-dz

Very nice, thank you for making those changes.

If you can also add an entry to the CHANGELOG.md file in crates/solana-cli like how the other entries are (you can put it under "Unreleased"), I can go ahead and approve this guy after the checks pass

RedaRahmani · 2025-11-25T15:14:05Z

Hi @karl-dz ,
I’ve added the entry to crates/solana-cli/CHANGELOG.md under Unreleased and resolved the changelog merge conflict.

Please let me know if you’d like any tweaks to the wording or placement.

karl-dz · 2025-11-25T15:19:03Z

Hi @karl-dz , I’ve added the entry to crates/solana-cli/CHANGELOG.md under Unreleased and resolved the changelog merge conflict.

Please let me know if you’d like any tweaks to the wording or placement.

420e06f and 420e06f can be removed if you rebase with main. Assuming the remote for this repo is upstream and you have reset your branch by taking out these commits:

git fetch upstream
git rebase upstream/main

You mind giving that a try? There may be a merge conflict at 94fa4c4 in the changelog when you do this, but that should be easy to resolve. Let me know if you have any issues

RedaRahmani · 2025-11-25T16:11:35Z

Hi @karl-dz , I rebased feat/passport-validator-access-validation onto upstream/main, resolved the CHANGELOG.md conflict, and force-pushed. The commits 94fa4c4 and 420e06f are no longer in the branch history. Please let me know if anything else looks off or if you’d like me to tweak the approach.

RedaRahmani · 2025-11-25T21:25:09Z

Hi @karl-dz

I’ve fixed the formatting issues (just fmt-check is now clean).

I also tried running sh/test_doublezero_solana_clean.sh locally.
The script currently fails at doublezero-solana passport fetch -ul --config with:

Error: Failed to fetch account F5hVnYTU4CeSxhyGQBKHQRZFtoCn5NBUNWMXP3T7giAR.

This seems related to the local fork / config account rather than the new validator access validation logic, but I’m happy to adjust the script if you’d like me to change how the test is set up.

karl-dz · 2025-11-26T02:54:40Z

Hi @karl-dz

I’ve fixed the formatting issues (just fmt-check is now clean).

I also tried running sh/test_doublezero_solana_clean.sh locally. The script currently fails at doublezero-solana passport fetch -ul --config with:

Error: Failed to fetch account F5hVnYTU4CeSxhyGQBKHQRZFtoCn5NBUNWMXP3T7giAR.

This seems related to the local fork / config account rather than the new validator access validation logic, but I’m happy to adjust the script if you’d like me to change how the test is set up.

I'll have to put up a PR to remove that shell script. Try to follow the workflow found in local-validator.yml: https://github.com/RedaRahmani/doublezero-offchain/blob/feat/passport-validator-access-validation/.github/workflows/local-validator.yml#L37-L42. Let me know if you have an issue using doublezero-solana-fork.

RedaRahmani · 2025-11-26T15:04:24Z

Hi @karl-dz , Thanks a lot for the detailed review!

I’ve re-run sh/test_doublezero_solana_clean.sh on my side following the local-validator.yml workflow (using doublezero-solana-fork with -um --reset --god-mode).

With that setup:

doublezero-solana passport fetch -ul --config works and prints the program config correctly. I couldn’t reproduce the Failed to fetch account F5hV… error once I followed the same steps as in local-validator.yml.

The part that now fails without any extra flags is

doublezero-solana passport request-validator-access ...

because the new validation correctly complains that the local validator is not an active staked validator, so the access-request account is never created. The final

doublezero-solana passport fetch -ul --access-request <service_key>

then fails with Failed to fetch account …, which is expected given the stricter checks.

If I add the new --force flag to the request-validator-access call in the script, the whole flow passes: the access request is created and the last fetch --access-request succeeds. So behaviour-wise the CLI changes seem correct; it’s really the shell script that needs to be adapted for the “no stake on local fork” case.

I’m happy to:

update sh/test_doublezero_solana_clean.sh in this PR to include --force for the local-validator scenario, or

drop the shell script and rely purely on the local-validator.yml workflow as you suggested.

Let me know which option you’d prefer and I’ll update the PR accordingly.
Could you also please confirm that I’m validating these new checks in the right way, or let me know if there’s any additional scenario you’d like me to cover?

karl-dz · 2025-11-26T18:48:10Z

Hi @karl-dz , Thanks a lot for the detailed review!

I’ve re-run sh/test_doublezero_solana_clean.sh on my side following the local-validator.yml workflow (using doublezero-solana-fork with -um --reset --god-mode).

With that setup:

doublezero-solana passport fetch -ul --config works and prints the program config correctly. I couldn’t reproduce the Failed to fetch account F5hV… error once I followed the same steps as in local-validator.yml.

The part that now fails without any extra flags is

doublezero-solana passport request-validator-access ...

because the new validation correctly complains that the local validator is not an active staked validator, so the access-request account is never created. The final

doublezero-solana passport fetch -ul --access-request <service_key>

then fails with Failed to fetch account …, which is expected given the stricter checks.

If I add the new --force flag to the request-validator-access call in the script, the whole flow passes: the access request is created and the last fetch --access-request succeeds. So behaviour-wise the CLI changes seem correct; it’s really the shell script that needs to be adapted for the “no stake on local fork” case.

I’m happy to:

update sh/test_doublezero_solana_clean.sh in this PR to include --force for the local-validator scenario, or

drop the shell script and rely purely on the local-validator.yml workflow as you suggested.

Let me know which option you’d prefer and I’ll update the PR accordingly. Could you also please confirm that I’m validating these new checks in the right way, or let me know if there’s any additional scenario you’d like me to cover?

Let me try something locally. Will keep you updated.

karl-dz · 2025-11-26T21:29:58Z

Hi @karl-dz , Thanks a lot for the detailed review!
I’ve re-run sh/test_doublezero_solana_clean.sh on my side following the local-validator.yml workflow (using doublezero-solana-fork with -um --reset --god-mode).
With that setup:
doublezero-solana passport fetch -ul --config works and prints the program config correctly. I couldn’t reproduce the Failed to fetch account F5hV… error once I followed the same steps as in local-validator.yml.
The part that now fails without any extra flags is
doublezero-solana passport request-validator-access ...
because the new validation correctly complains that the local validator is not an active staked validator, so the access-request account is never created. The final
doublezero-solana passport fetch -ul --access-request <service_key>
then fails with Failed to fetch account …, which is expected given the stricter checks.
If I add the new --force flag to the request-validator-access call in the script, the whole flow passes: the access request is created and the last fetch --access-request succeeds. So behaviour-wise the CLI changes seem correct; it’s really the shell script that needs to be adapted for the “no stake on local fork” case.
I’m happy to:
update sh/test_doublezero_solana_clean.sh in this PR to include --force for the local-validator scenario, or
drop the shell script and rely purely on the local-validator.yml workflow as you suggested.
Let me know which option you’d prefer and I’ll update the PR accordingly. Could you also please confirm that I’m validating these new checks in the right way, or let me know if there’s any additional scenario you’d like me to cover?

Let me try something locally. Will keep you updated.

We just merged a fix to the leader schedule evaluation. Can you rebase with the latest main? I just rebased with main locally in your branch and it seems to work now (so no need to pass in --force)

karl-dz · 2025-11-26T21:35:58Z

Hi @karl-dz , Thanks a lot for the detailed review!
I’ve re-run sh/test_doublezero_solana_clean.sh on my side following the local-validator.yml workflow (using doublezero-solana-fork with -um --reset --god-mode).
With that setup:
doublezero-solana passport fetch -ul --config works and prints the program config correctly. I couldn’t reproduce the Failed to fetch account F5hV… error once I followed the same steps as in local-validator.yml.
The part that now fails without any extra flags is
doublezero-solana passport request-validator-access ...
because the new validation correctly complains that the local validator is not an active staked validator, so the access-request account is never created. The final
doublezero-solana passport fetch -ul --access-request <service_key>
then fails with Failed to fetch account …, which is expected given the stricter checks.
If I add the new --force flag to the request-validator-access call in the script, the whole flow passes: the access request is created and the last fetch --access-request succeeds. So behaviour-wise the CLI changes seem correct; it’s really the shell script that needs to be adapted for the “no stake on local fork” case.
I’m happy to:
update sh/test_doublezero_solana_clean.sh in this PR to include --force for the local-validator scenario, or
drop the shell script and rely purely on the local-validator.yml workflow as you suggested.
Let me know which option you’d prefer and I’ll update the PR accordingly. Could you also please confirm that I’m validating these new checks in the right way, or let me know if there’s any additional scenario you’d like me to cover?

Let me try something locally. Will keep you updated.

We just merged a fix to the leader schedule evaluation. Can you rebase with the latest main? I just rebased with main locally in your branch and it seems to work now (so no need to pass in --force)

Actually, I am mistaken. If you happen to rebase after this PR gets merged, CI will fail again. But that is because the default number of epochs is 2.

If you add another hidden optional argument for request access (something like --leader-schedule-epochs), where you pass in --leader-schedule-epochs 1 in the test shell script, this will succeed. Would you be able to do that? Thank you for being such a good sport with this 😂

RedaRahmani · 2025-11-27T00:45:45Z

Hi @karl-dz , Haha, no worries at all , I’m happy to be a good sport about it 😄

I’ll add a hidden --leader-schedule-epochs argument to request-validator-access and update sh/test_doublezero_solana_clean.sh to call it with --leader-schedule-epochs 1.

I’m away from my desk right now, but I’ll make these changes once I’m back home and will let you know when the updates are pushed.

karl-dz · 2025-11-27T01:19:47Z

Hi @karl-dz , Haha, no worries at all , I’m happy to be a good sport about it 😄

I’ll add a hidden --leader-schedule-epochs argument to request-validator-access and update sh/test_doublezero_solana_clean.sh to call it with --leader-schedule-epochs 1.

I’m away from my desk right now, but I’ll make these changes once I’m back home and will let you know when the updates are pushed.

Adding this argument to the prepare command as well would be good, too, if you can. And since there is so much overlap with arguments, it may also make sense to have a shared commands struct in the submodule you added and to flatten that struct into prepare and request. Up to you, though. Thank you

RedaRahmani · 2025-11-27T14:46:44Z

Hi @karl-dz , I’ve implemented the changes you suggested

Added a shared SharedAccessArgs struct in the passport submodule and flattened it into both prepare and request, as you proposed.
Included the hidden --leader-schedule-epochs argument (default 2) for both prepare-validator-access and request-validator-access.
Passed leader_schedule_epochs through to the validation helper so the leader schedule checks use this value.
Updated sh/test_doublezero_solana_clean.sh to call --leader-schedule-epochs 1.
I also followed the local-validator.yml workflow you pointed me to , doublezero-solana-fork is running fine and the test script is passing locally, so CI should be happy now

Please let me know if anything looks off or if you’d like me to adjust anything further.

karl-dz · 2025-11-27T20:35:37Z

Hi @karl-dz , I’ve implemented the changes you suggested

Added a shared SharedAccessArgs struct in the passport submodule and flattened it into both prepare and request, as you proposed.

Included the hidden --leader-schedule-epochs argument (default 2) for both prepare-validator-access and request-validator-access.

Passed leader_schedule_epochs through to the validation helper so the leader schedule checks use this value.

Updated sh/test_doublezero_solana_clean.sh to call --leader-schedule-epochs 1.

I also followed the local-validator.yml workflow you pointed me to , doublezero-solana-fork is running fine and the test script is passing locally, so CI should be happy now

Please let me know if anything looks off or if you’d like me to adjust anything further.

Nice. I took quick look through the changes and have a few suggestions.

Instead of defaulting to the literal 2 for the command argument, can we use Option instead? Then you can unwrap_or(ENV_PREVIOUS_LEADER_EPOCHS) in your module. It would be good to preserve this constant instead of having the number 2 look like a magic number.
Instead of pulling down the commits from main on top of your commits, can you remove those commits and instead fetch and rebase like you did before?

This change looks like it is almost ready to ship. Good job with everything so far.

…IOUS_LEADER_EPOCHS default

RedaRahmani · 2025-11-27T21:28:06Z

Hi @karl-dz , Thanks a lot for the review and the kind words!

I’ve updated the PR based on your suggestions:

Switched the command argument to Option and now use unwrap_or(ENV_PREVIOUS_LEADER_EPOCHS) in the module instead of the literal 2, so the default constant is preserved and we avoid the magic number.

Cleaned up the history by dropping the extra merge commits and rebasing on top of the latest main like before.

Please let me know if you’d like any further tweaks.

karl-dz

Thank you for addressing this issue!

RedaRahmani · 2025-11-27T23:25:50Z

Thank you so much for the guidance and quick reviews, really appreciate it!
Would be happy to help with more issues anytime. 🙌

karl-dz · 2025-11-28T01:51:53Z

Thank you so much for the guidance and quick reviews, really appreciate it! Would be happy to help with more issues anytime. 🙌

Of course. We usually track our issues here, so feel free to continue contributing: https://github.com/malbeclabs/doublezero/issues. We appreciate the help

…blezero-offchain#211) ## Summary of Changes This PR implements the behavior described in malbeclabs/doublezero-offchain#155 for the passport CLI: - Extracts the validator access validation (gossip + leader scheduler checks for primary and backup validators) into a shared helper module: `crates/solana-cli/src/command/passport/access_validation.rs`. - Refactors `prepare-validator-access` to use this shared helper, preserving the existing output and `--force` semantics. - Extends `request-validator-access` to: - print the same cluster / validator info as `prepare-validator-access`, - run the same validation checks before PDA/signature/tx submission, and - support a new `-f/--force` flag so operators can override validation while still seeing the errors. - Wires the new module into `crates/solana-cli/src/command/passport/mod.rs`. With this change, `request-validator-access` now mirrors the validation behavior of `prepare-validator-access`: - Without `--force`: validation errors stop execution before the transaction is sent. - With `--force`: validation errors are printed as warnings and the transaction is still submitted. ## Testing Verification I ran the CLI crate tests locally: ```bash cargo test -p doublezero-solana-cli

karl-dz suggested changes Nov 24, 2025

View reviewed changes

karl-dz mentioned this pull request Nov 26, 2025

fetching leader schedule in sentinel for local validator instance is unusable malbeclabs/doublezero#2306

Closed

RedaRahmani added 6 commits November 27, 2025 20:52

Add shared validator access validation and force flag

a49ab3d

Address review feedback on passport access validation

42527d5

Update changelog for passport validator access validation

773662c

Fix formatting in passport access validation module

12a4b7f

refactor(passport): share access args & leader schedule epochs

dc229f1

Refine previous_leader_epochs CLI arg to use Option and keep ENV_PREV…

878a83a

…IOUS_LEADER_EPOCHS default

karl-dz approved these changes Nov 27, 2025

View reviewed changes

karl-dz merged commit 4a09cb1 into malbeclabs:main Nov 27, 2025
2 checks passed

RedaRahmani deleted the feat/passport-validator-access-validation branch November 28, 2025 15:46

karl-dz mentioned this pull request Dec 1, 2025

Add validation checks to request-validator-access #155

Closed

Uh oh!

Conversation

RedaRahmani commented Nov 24, 2025

Summary of Changes

Testing Verification

Uh oh!

RedaRahmani commented Nov 24, 2025

Uh oh!

karl-dz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RedaRahmani commented Nov 24, 2025

Uh oh!

karl-dz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RedaRahmani commented Nov 25, 2025

Uh oh!

karl-dz commented Nov 25, 2025

Uh oh!

RedaRahmani commented Nov 25, 2025

Uh oh!

RedaRahmani commented Nov 25, 2025

Uh oh!

karl-dz commented Nov 26, 2025

Uh oh!

RedaRahmani commented Nov 26, 2025

Uh oh!

karl-dz commented Nov 26, 2025

Uh oh!

karl-dz commented Nov 26, 2025

Uh oh!

karl-dz commented Nov 26, 2025

Uh oh!

RedaRahmani commented Nov 27, 2025

Uh oh!

karl-dz commented Nov 27, 2025

Uh oh!

RedaRahmani commented Nov 27, 2025

Uh oh!

karl-dz commented Nov 27, 2025

Uh oh!

RedaRahmani commented Nov 27, 2025

Uh oh!

karl-dz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RedaRahmani commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

karl-dz commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

karl-dz left a comment •

edited

Loading

RedaRahmani commented Nov 27, 2025 •

edited

Loading